Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Final Capstone Project: Rent-Eaze Booking System - Merge Request #55

Merged
merged 153 commits into from
Nov 16, 2023
Merged

Conversation

lRebornsl
Copy link
Collaborator

@lRebornsl lRebornsl commented Nov 15, 2023

Overview
This pull request is a formal request to merge our development team's cumulative and concerted efforts from the dev branch into the main branch. Our Final Capstone Project is a sophisticated rental house booking system, designed and tailored following the prototype provided by Murat Korkmaz on Behance, with our unique spin - instead of motorcycles, we facilitate the booking of rental houses.

Our development team, composed of Javier Grau, Manuel Sanchez, and Anthony Vásquez, has meticulously adhered to coding best practices, proper Gitflow, and comprehensive professional documentation throughout the project lifecycle.

Pull Request from front-end

General Requirements Met

  • Zero linter errors across the codebase.
  • Strict adherence to Gitflow practices.
  • Professional-level documentation and code commenting.

Project Implementation Details

Chosen Theme

  • Our application is built around the concept of booking rental houses, aiming to streamline the process of scheduling a house with an intuitive user interface.

Core Features Implemented

  • Normal user login system by typing the email and password.
  • The navigation panel provides seamless access to:
    • List of houses.
    • House reservation form.
    • Functionality to add new houses to the system.
    • Option to remove their own houses from the system.
  • The main page features a listing of houses available for users.
  • Detail pages for each house, with an option for users to reserve it.
  • A fully responsive design, with tailored user experiences for both mobile and desktop versions.

Additional Features (Beyond Basic Requirements)

  • The reservation process is enhanced by allowing users to select only a future date.

Comprehensive Documentation

  • Detailed API documentation has been included to facilitate understanding and potentially scaling the application in the future.

Useful Links

Merge Request Justification
With all required features meticulously implemented, we are confident that our project meets the specified criteria. We kindly request the review and approval for the merging of our dev branch into main.

We eagerly await any feedback and are fully prepared to conduct further modifications if recommended by our esteemed reviewers.

Thank you for your consideration and time.

Warm regards,
@grauJavier
@Luffytaro22
@lRebornsl

Copy link

@Meltrust Meltrust left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @lRebornsl,

Good job so far!
There are some issues that you still need to work on to prepare your project for the final evaluation, but you are almost there!

To highlight:

  • All tests but 1 are passing✔️
  • Nice code organization ✔️
  • API is working well✔️
  • Good documentation✔️

You are really close to finishing the Microverse program!! Keep it up! 👍👍👍

You can do it

After implementing the requested changes, please submit another review request. ♻️

Check the comments under the review.

Cheers and Happy coding!👏👏👏

Please, do not open a new Pull Request for re-reviews. You should use the same Pull Request submitted for the previous reviews unless it is requested otherwise.


Comment on lines +178 to +182

rspec

```

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • When I run the tests according to instructions, I get this failing test:

image

It seems that additional instructions are required in your readme to deal with the JWT verification key. Kindly, add those to your readme. 👍

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello!
Thanks for your feedback 🙌🏼
This is solved 😄

README.md Outdated
Comment on lines 139 to 144

rails db:create
rails db:migrate

```
Copy link

@Meltrust Meltrust Nov 15, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • You have included a functioning seeds.rb file. Kindly, include in your instructions the command rails db:seed.

That way, your users will enjoy a pre-seeded database. 👍

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello!
Thanks for your feedback 🙌🏼
This is solved 😄

Comment on lines 42 to 46
def destroy
place = @places.find(params[:id])
place.destroy
head :no_content
end

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • What happens here if the place is not successfully destroyed? Kindly, add some error handling like you did on the update method (line 32). 👍

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello!
Thanks for your feedback 🙌🏼
This is solved 😄

Comment on lines 49 to 53
def destroy
reservation = @reservations.find(params[:id])
reservation.destroy
head :no_content
end

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • What happens here if the reservation is not successfully destroyed? Kindly, add some error handling like you did on the update method (line 39). 👍

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello!
Thanks for your feedback 🙌🏼
This is solved 😄

Copy link

@Meltrust Meltrust left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @lRebornsl,

Good job so far!
There are some issues that you still need to work on to prepare your project for the final evaluation, but you are almost there!

You are really close to finishing the Microverse program!! Keep it up! 👍👍👍

You can do it

After implementing the requested changes, please submit another review request. ♻️

Check the comments under the review.

Cheers and Happy coding!👏👏👏

Please, do not open a new Pull Request for re-reviews. You should use the same Pull Request submitted for the previous reviews unless it is requested otherwise.


```sh

rails db:create

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • In this review iteration, when I try to create the database according to instructions, this happens:

image

Explanation

Problem 1:

The first problem is this little bang ("!"):

image

If I remove it, then the error becomes this:

Second problem

image

The new problem is that the JWT secret token is missing from rails credentials.

These are the steps to solve this second problem:

1. Remove config/master.key and config/credentials.yml.enc if they exist.
2. Run `rails secret`.  Copy the key.
3. Run EDITOR="code --wait" bin/rails credentials:edit
4. In the editor that opens, add this:  devise_jwt_secret_key: <the key you copied in step 2>
5. Save the file and close the editor.  New master.key, credentials.yml.enc files will be generated, and the key will be stored in `Rails.application.credentials.devise_jwt_secret_key`.

Result:

image

Now it works.

This may be working for you on your end because you have the database and credentials already setup. But it won't for new users.

Therefore:

  1. Kindly fix the first problem in config/initializers/devise.rb (the bang "!").
  2. Next, please include instructions on your readme about how to add the devise_jwt_secret_key to the rails credentials, (these instructions have already been written for you above).

That way, the database will be able to be created, and all your tests will pass:

image

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello!
Thanks for your feedback 🙌🏼
This is solved 😄

Copy link

@Meltrust Meltrust left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @lRebornsl,

Wow, you did it 🎉

Brilliant

Thank you for the changes implemented 💪 🥇 ㊗️

Unless you want to add more features, go ahead to your final presentation ⏩ ⏩ ⏩

You are about to finish the Microverse program. You have come a long way!!!

Good luck in the software industry!! I'll see you there. ✨

Congratulations!!!!!! 🎉

applause

To highlight

  • Readme instructions have been improved✔️
  • All tests are now passing✔️
  • Great job✔️

Cheers and Happy coding!👏👏👏


@Luffytaro22 Luffytaro22 merged commit 0eff724 into main Nov 16, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants